Fix write to Nimble table in HiveConnector #10941
Conversation
facebook-github-bot
added
the
CLA Signed
✅ Deploy Preview for meta-velox canceled.
|
|
This pull request was exported from Phabricator. Differential Revision: D62302602 |
| @@ -993,12 +993,20 @@ void updateWriterOptionsFromHiveConfig( | |||
| const std::shared_ptr<const HiveConfig>& hiveConfig, | |||
| const config::ConfigBase* sessionProperties, | |||
| std::shared_ptr<dwio::common::WriterOptions>& writerOptions) { | |||
| if (fileFormat == dwio::common::FileFormat::PARQUET) { | |||
| switch (fileFormat) { | |||
There was a problem hiding this comment.
@Yuhta should we make this a virtual call to remove the switch?
There was a problem hiding this comment.
It's not possible because the virtual call would be in file readers/writers and we need the Hive information here. File formats cannot depend on Hive but the reverse is good.
There was a problem hiding this comment.
@Yuhta Is there an actual dependency on Hive in these functions? It seems like all they are doing is fetching configs, and the parsing and fetching of these configs probably belongs with the file writer library itself. Similar to what we did for other parquet configs recently.
There was a problem hiding this comment.
It seems our change to WriterOptions is refactored in the mentioned PR. Let's merge this to unblock clients and address the dispatch problem in subsequent diff. One possible solution is to move only the Hive config keys (this should be only one header file and dependency free) into dwio/common, since dw here is almost equivalent to Hive. CC: @xiaoxmeng
|
This pull request was exported from Phabricator. Differential Revision: D62302602 |
Summary: Pull Request resolved: facebookincubator#10941 Recent refactor facebookincubator#10915 breaks the capability to write to Nimble table in Hive connector, fix it. Reviewed By: xiaoxmeng Differential Revision: D62302602
Yuhta
force-pushed
the
export-D62302602
branch
from
0a192d6
to
19fd440
Compare
|
This pull request was exported from Phabricator. Differential Revision: D62302602 |
Summary: Pull Request resolved: facebookincubator#10941 Recent refactor facebookincubator#10915 breaks the capability to write to Nimble table in Hive connector, fix it. Reviewed By: xiaoxmeng Differential Revision: D62302602
Yuhta
force-pushed
the
export-D62302602
branch
from
19fd440
to
7a6f0b7
Compare
|
This pull request was exported from Phabricator. Differential Revision: D62302602 |
Summary: Pull Request resolved: facebookincubator#10941 Recent refactor facebookincubator#10915 breaks the capability to write to Nimble table in Hive connector, fix it. Reviewed By: xiaoxmeng Differential Revision: D62302602
Yuhta
force-pushed
the
export-D62302602
branch
from
7a6f0b7
to
59d4276
Compare
Summary: Pull Request resolved: facebookincubator#10941 Recent refactor facebookincubator#10915 breaks the capability to write to Nimble table in Hive connector, fix it. Reviewed By: xiaoxmeng Differential Revision: D62302602
|
This pull request was exported from Phabricator. Differential Revision: D62302602 |
Yuhta
force-pushed
the
export-D62302602
branch
from
59d4276
to
2e84c5a
Compare
|
This pull request has been merged in d9e0e9b. |
|
Conbench analyzed the 1 benchmark run on commit There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
Summary:
Recent refactor #10915
breaks the capability to write to Nimble table in Hive connector, fix it.
Differential Revision: D62302602